Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Turning off logger at the class-constructor level #323

Merged
merged 20 commits into from
Sep 25, 2024
Merged

Turning off logger at the class-constructor level #323

merged 20 commits into from
Sep 25, 2024

Conversation

aadityasinha-dotcom
Copy link
Contributor

What It Does

Resolves #316

How to Test

Review Checklist
I certify that I have:

Additional Comments

@aadityasinha-dotcom
turning off logger for SdkApi
9d3b834 
Signed-off-by: aadityasinha-dotcom <[email protected]>
@aadityasinha-dotcom
fix test failing
9571c8c 
Signed-off-by: aadityasinha-dotcom <[email protected]>
@aadityasinha-dotcom aadityasinha-dotcom marked this pull request as ready for review August 21, 2024 17:48
t1m0thyj
t1m0thyj requested changes Aug 23, 2024
View reviewed changes
Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aadityasinha-dotcom! Left a few comments

src/core/zowe/core_for_zowe_sdk/sdk_api.py Outdated Show resolved Hide resolved
src/zos_console/zowe/zos_console_for_zowe_sdk/console.py Outdated Show resolved Hide resolved
@aadityasinha-dotcom
changes to the SdkApi classes
420388e 
Signed-off-by: aadityasinha-dotcom <[email protected]>
traeok
traeok approved these changes Aug 27, 2024
View reviewed changes
Copy link
Member

@traeok traeok left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @aadityasinha-dotcom for the enhancement!
When you get a chance, can you please add an entry to the changelog to track this change?

t1m0thyj
t1m0thyj previously requested changes Aug 28, 2024
View reviewed changes
src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py Outdated Show resolved Hide resolved
src/core/zowe/core_for_zowe_sdk/sdk_api.py Outdated Show resolved Hide resolved
src/core/zowe/core_for_zowe_sdk/sdk_api.py Show resolved Hide resolved
@aadityasinha-dotcom
changes
b3dfaed 
Signed-off-by: aadityasinha-dotcom <[email protected]>
@aadityasinha-dotcom
added test for logger
bb74d88 
Signed-off-by: aadityasinha-dotcom <[email protected]>
@traeok
Copy link
Member

traeok commented Sep 6, 2024

Looks like the log parameter is missing from the docstrings in a few functions - see this stage for more info.

Please resolve the lint errors when possible - thanks 🙂

@t1m0thyj t1m0thyj self-requested a review September 16, 2024 16:24
@aadityasinha-dotcom
Copy link
Contributor Author

Hello, I have resolved the lint errors also, please review it ☺️

@codecov Codecov
Copy link

codecov bot commented Sep 18, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.07%. Comparing base (b00a4c1) to head (de45309).
Report is 21 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
+ Coverage   89.05%   89.07%   +0.01%     
==========================================
  Files          65       65              
  Lines        3244     3249       +5     
==========================================
+ Hits         2889     2894       +5     
  Misses        355      355
Flag Coverage Δ
unittests 89.07% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

traeok
traeok approved these changes Sep 18, 2024
View reviewed changes
Copy link
Member

@traeok traeok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @aadityasinha-dotcom!

zFernand0
zFernand0 approved these changes Sep 23, 2024
View reviewed changes
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the implementation @aadityasinha-dotcom

I think it'd be good to have a quick Changelog entry about this. 🥳

## Recent Changes

### Enhancements

- Turning of ......... [#316] (https://github.com/zowe/zowe-client-python-sdk/issues/316)

LGTM! 😋

@aadityasinha-dotcom
changelog
0a1020a 
Signed-off-by: aadityasinha-dotcom <[email protected]>
aadityasinha-dotcom
aadityasinha-dotcom commented Sep 24, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@aadityasinha-dotcom
changes
de45309 
Signed-off-by: aadityasinha-dotcom <[email protected]>
@zFernand0 zFernand0 dismissed t1m0thyj’s stale review September 25, 2024 13:25

I believe the changes were addressed 🙏

@zFernand0 zFernand0 merged commit 64bc3ca into zowe:main Sep 25, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zFernand0 zFernand0 zFernand0 approved these changes

@traeok traeok traeok approved these changes

@awharn awharn Awaiting requested review from awharn

@t1m0thyj t1m0thyj Awaiting requested review from t1m0thyj

Assignees
No one assigned
Labels
None yet
Projects
Zowe CLI Squad
Status: Closed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: Allow logger to be disabled at the class-constructor level
4 participants
@aadityasinha-dotcom @traeok @t1m0thyj @zFernand0